Skip to content

Conversation

@coolljt0725
Copy link
Member

based on the runtime spec, the user are platform-specific and MemorySwap are for Linux based systems.

Fixed #329

Actually, I'd like to remove Memory MemorySwap CpuShares

@wking
Copy link
Contributor

wking commented Oct 10, 2016

On Sat, Oct 08, 2016 at 02:33:13AM -0700, Lei Jitang wrote:

based on the runtime spec, the user are platform-specific and
MemorySwap are for Linux based systems.

Solaris also supports the user fields 1. Both Solaris 2 and
Windows 3 have similar-ish memory settings, but they aren't quite
MemorySwap.

Actually, I'd like to remove Memory MemorySwap CpuShares

I'd rather use the runtime-spec config.json [4,5,6]. It saves us from
having to copy over settings from runtime-spec, and leaves the “what
settings do I think are safe?” descision up to the image-consumer (who
can use a stock sanitization script if they want).

@coolljt0725
Copy link
Member Author

@wking

Solaris also supports the user fields [1]. Both Solaris [2] and
Windows [3] have similar-ish memory settings, but they aren't quite
MemorySwap.

Solaris does have user, I didn't say that Solaris doesn't user field. but we doesn't clarify that the image spec is support for Solaris at the moment :)
Windows and Solaris do have memory limit, but it's not MemorySwap

@wking
Copy link
Contributor

wking commented Oct 11, 2016

On Tue, Oct 11, 2016 at 01:41:09AM -0700, Lei Jitang wrote:

Solaris also supports the user fields [1]. Both Solaris [2] and
Windows [3] have similar-ish memory settings, but they aren't
quite MemorySwap.

Solaris does have user, I didn't say that Solaris doesn't user
field. but we doesn't clarify that the image spec is support for
Solaris at the moment :)

As long as we have an image-spec-specific config with a user field,
I'd rather clarify it by requiring config translators to populate the
runtime-spec config's process.user for all runtime-spec OSes. Solaris
and Linux have the same runtime-spec config, so I'd expect they'd have
the same image-spec-config support.

Windows and Solaris do have memory limit, but it's not MemorySwap

As long as we continue to support an image-spec-specific MemorySwap
setting, I think it makes sense to add support for the related Solaris
and Windows fields. On the other hand, if we decide to drop
MemorySwap, then we can skip the related Solaris and Windows fields.
But it seems inconsistent to only distribute memory/swap limits if the
target OS happens to be Linux.

@vbatts
Copy link
Member

vbatts commented Oct 19, 2016

Largely, LGTM
I am in favor of just removing Memory MemorySwap CpuShares too

@vbatts
Copy link
Member

vbatts commented Nov 1, 2016

needs a rebase

@coolljt0725 coolljt0725 force-pushed the config_platform_specific branch from de8096c to acbc797 Compare November 2, 2016 03:05
@coolljt0725
Copy link
Member Author

@vbatts rebased

@vbatts
Copy link
Member

vbatts commented Nov 2, 2016

LGTM

Approved with PullApprove

@jonboulle
Copy link
Contributor

jonboulle commented Nov 4, 2016

lgtm for now

Approved with PullApprove

@jonboulle jonboulle merged commit 7cab1f8 into opencontainers:master Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants